Skip to content

feat: trigger skill review separately#191

Merged
louisliu2048 merged 2 commits intomainfrom
vui-chee/skill-trigger-separately
Mar 12, 2026
Merged

feat: trigger skill review separately#191
louisliu2048 merged 2 commits intomainfrom
vui-chee/skill-trigger-separately

Conversation

@Vui-Chee
Copy link
Contributor

No description provided.

@louisliu2048 louisliu2048 merged commit ddb2b2b into main Mar 12, 2026
2 checks passed
Comment on lines +68 to +74
- name: Run Claude Review
id: claude
uses: anthropics/claude-code-action@v1
with:
anthropic_api_key: ${{ secrets.ANTHROPIC_API_KEY }}
prompt: |
${{ steps.skill.outputs.skill_content }}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 The claude-review job passes the review skill content via the prompt input (line 73-74), which causes claude-code-action to enter agent mode instead of tag mode for comment-triggered events. In agent mode, no PR diff or conversation history is pre-loaded, MCP servers for GitHub interaction are degraded or absent, and no tracking comment is posted—so Claude receives only the generic review.md instructions with no actual PR context to review. Consider removing prompt and using trigger_phrase: "@review" instead so the action stays in tag mode, which automatically provides full PR context, MCP tools, and tracking comments.

Extended reasoning...

What the bug is

The claude-review job (lines 39-74) is designed to perform code reviews when someone comments @review on a PR. It loads the review skill from .claude/skills/review.md and passes it via the prompt input to anthropics/claude-code-action@v1. However, providing the prompt input fundamentally changes how the action operates.

How it manifests

In claude-code-action, the detectMode() function in src/modes/detector.ts checks whether the prompt input is provided for comment events. When it is truthy, the function returns "agent" mode instead of "tag" mode. This has been confirmed by multiple reviewers reading the actual source code:

if (isIssueCommentEvent(context) || isPullRequestReviewCommentEvent(context) || ...) {
  if (context.inputs.prompt) {
    return "agent";
  }
}

Since the claude-review job always provides prompt with the skill content, it will always enter agent mode for comment-triggered events.

Why existing code doesn't prevent it

Compare the two jobs: the claude job (lines 14-37) provides no prompt input, so it operates in tag mode — the fully-featured mode that calls fetchGitHubData() to build a rich prompt with PR diff, conversation history, and metadata, configures MCP servers with GitHub interaction tools, and creates tracking comments for user feedback. The claude-review job was presumably intended to work the same way but with review-specific instructions. Instead, by passing prompt, it accidentally opts into agent mode which skips all of this context loading.

Additionally, the claude-review job only has contents: read permissions (vs contents: write for the claude job), further limiting what Claude could accomplish even if it found workarounds.

Step-by-step proof

  1. A user comments @review on a PR.
  2. The claude-review job triggers (lines 40-41 condition matches).
  3. The Load review skill step reads .claude/skills/review.md and sets it as skill_content output.
  4. The Run Claude Review step calls claude-code-action@v1 with prompt set to the review skill content.
  5. Inside the action, detectMode() sees prompt is truthy on a comment event and returns "agent".
  6. Agent mode runs: it writes only the review.md content to the prompt file. It does NOT call fetchGitHubData(), does NOT create a tracking comment, and configures MCP with empty allowedTools (since no claude_args is provided).
  7. Claude receives instructions like "Read the diff or changed files" but has no diff, no PR number, no conversation context, and no GitHub MCP tools to fetch them.
  8. The review is effectively non-functional.

How to fix it

Replace the prompt input with trigger_phrase: "@review" and optionally pass the skill content via custom_instructions. This keeps the action in tag mode for comment events, providing full PR context, MCP servers, and tracking comments automatically. Claude will discover the review skill from the repo checkout via .claude/skills/review.md. The separate Load review skill step can be removed entirely.

Comment on lines +40 to +42
if: |
(github.event_name == 'issue_comment' && contains(github.event.comment.body, '@review')) ||
(github.event_name == 'pull_request_review_comment' && contains(github.event.comment.body, '@review'))
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 The claude-review trigger conditions have a few gaps: (1) issue_comment fires for both issues and PRs, so @review on a regular issue triggers a code review job with no diff to review — adding && github.event.issue.pull_request would fix this; (2) a comment containing both @claude and @review triggers both jobs simultaneously, producing duplicate work; (3) the pull_request_review (submitted) event is handled by the claude job but not claude-review, so @review in a review summary is silently ignored.

Extended reasoning...

The issue_comment event fires for regular issues too

In GitHub Actions, the issue_comment event fires for comments on both issues and pull requests (since PRs are a type of issue). The claude-review job condition on line 41 checks:

github.event_name == 'issue_comment' && contains(github.event.comment.body, '@review')

This means if someone writes @review on a regular issue (not a PR), the job will trigger and attempt to run a code review via claude-code-action with the review.md prompt. Since there is no diff or changed files on a regular issue, the action will either fail or produce nonsensical output, wasting CI resources.

The fix is straightforward: add && github.event.issue.pull_request to the issue_comment condition to ensure it only fires on PR comments. The original claude job can get away without this check because it is a general-purpose assistant that can handle issue interactions, but claude-review is specifically a code review tool that requires a PR context.

Double triggering with @claude

The claude job triggers on contains(comment.body, '@claude') and claude-review triggers on contains(comment.body, '@review'). These conditions are non-exclusive. A comment like "@claude please @review this PR" would trigger both jobs simultaneously, producing duplicate review comments. One could argue this is intentional (the user asked for both), but it is still worth noting since the two jobs would produce overlapping code review output — the claude job would see @claude and respond generically, while claude-review would run the dedicated review skill, leading to redundant and potentially conflicting feedback.

Missing pull_request_review event

The workflow-level on: block includes pull_request_review: types: [submitted] (line 10-11), and the claude job handles this event type (line 18). However, the claude-review job only handles issue_comment and pull_request_review_comment — it does not check for @review in the overall review submission body. This is a minor inconsistency since requesting an automated review from within your own review submission is an uncommon workflow, but it breaks the symmetry with the claude job pattern.

Concrete example for the issue_comment bug

Step-by-step proof for the most actionable issue:

  1. A user opens issue build: fix docker build, align cargo chef base image with latest rust toolchain version #100 (a regular issue, not a PR)
  2. Someone comments on issue build: fix docker build, align cargo chef base image with latest rust toolchain version #100 with the text: "@review this approach"
  3. The issue_comment event fires with github.event_name == 'issue_comment'
  4. The condition contains(github.event.comment.body, '@review') evaluates to true
  5. The claude-review job starts, checks out the repo, loads review.md, and invokes claude-code-action with a review prompt — but there is no PR diff to review
  6. The action either fails or produces a confused response on a regular issue

Suggested fix

Change line 41 from:

(github.event_name == 'issue_comment' && contains(github.event.comment.body, '@review'))

to:

(github.event_name == 'issue_comment' && github.event.issue.pull_request && contains(github.event.comment.body, '@review'))

Optionally, also add a pull_request_review condition and make the @review condition exclude @claude (or vice versa) to address the other two issues.

Vui-Chee added a commit that referenced this pull request Mar 13, 2026
* main: (25 commits)
  fix: bump quinn-proto to 0.11.14 to patch CVE-2026-31812 DoS vuln (#183)
  pre-job authorization (#193)
  fix: trigger review skill failed to ack (#192)
  feat: trigger skill review separately (#191)
  feat: add Claude skills and CLAUDE.md for AI-assisted development (#190)
  rename ext (#185)
  supply workflow for claude (#184)
  feat(builder): incremental trie cache optimization for flashblocks state root (#163)
  chore(flashblocks-rpc): migrate op-reth flashblocks into xlayer-reth (#175)
  Feat(chainspec): adding xlayer-devnet chainspec (#167)
  chore(builder): flatten flashblocks builder, remove unnecessary trait interfaces (#172)
  rpc: remove unnecessary trait bounds and dependencies from XlayerRpcExtApiServer impl (#171)
  fix fmt in bin/tools/gen_genesis.rs (#170)
  fix(builder): Resolve bugs on upstream flashblocks timing scheduler (#169)
  Feat(tools): Add a tool to generate a custom genesis file based on a template and existing chain data (#159)
  feat(flashblocks): Add flashblocks sequence persistence logic on RPC and sequence replay flashblock builder (#162)
  chore(builder): remove unused custom-engine-api feature flag in tests (#168)
  fix: p2p test hang due to hang on port (#165)
  fix: update testcontainers to v0.27.0 to remediate CVE-2025-62518 (#164)
  chore(builder): further clean up builder crate (#161)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants